Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk/dotnet): rework on query builder #7208

Closed
wants to merge 8 commits into from

Conversation

wingyplus
Copy link
Contributor

In this PR try to modernize DotNet SDK by do the following:

  • Upgrade .net from 7 to 8.
  • Setup .editorconfig to make dotnet format works.
  • Rework on query builder by introduce QueryBuilder class and port the logic from Go to it. Mostly feature is covered I guess.
  • Restructure folder from DaggerSDK to Dagger.SDK.
  • Temporary move DaggerSDKCodegen out of the solution. We will tackle next after this PR. :)

Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]>
Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]>
But still out of the solution since it under construction.

Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]>
@wingyplus wingyplus requested a review from a team as a code owner April 26, 2024 16:12
{
private readonly HttpClient _httpClient;

public GraphQLClient() : this(Environment.GetEnvironmentVariable("DAGGER_SESSION_PORT")!, Environment.GetEnvironmentVariable("DAGGER_SESSION_TOKEN")!)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our first priority is to get it support module. So I think it's ok for now to support only run under dagger run.


public class Dagger
{
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it empty class for now until we make codegen work.

Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]>
Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]>
Signed-off-by: Thanabodee Charoenpiriyakij <[email protected]>
@MikaelElkiaer
Copy link

@wingyplus This is funny coincidence. I just forked and started looking at code generation the other day: https://github.com/MikaelElkiaer/dagger/tree/feat/sdk-dotnet-codegen/sdk/dotnet
I was about to look at actually generating some queries.

@wingyplus
Copy link
Contributor Author

@MikaelElkiaer yeah, I just start exploring the codegen yesterday too. 😃

Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@pjmagee
Copy link
Contributor

pjmagee commented May 31, 2024

Would we benefit from using the built in .NET Source Generators? It's Roslyn Powered so can generate code at a compile/build time and can have external files fed into the code generator like json files, config files, query responses etc

{
var query = QueryBuilder
.Builder()
.Select("container")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will end up something like the Go SDK?

var container = dagger.Client.Container() <- QueryBuilder

container.From("image").WithEnv("...").WithExec("...")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the SDK API will construct the graphql query throuh this builder.


public class QueryBuilderTest
{
[Fact]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the original was using NUnit, and I've been using XUnit for years. Thoughts on https://devblogs.microsoft.com/dotnet/introducing-mstest-sdk/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pjmagee sorry for late reply. I think it would be great to migrate to it. I add an issue on dagger-dotnet-sdk about this wingyplus/dagger-dotnet-sdk#9

@pjmagee
Copy link
Contributor

pjmagee commented May 31, 2024

I was also looking into the SDK Docs on how to build an SDK. I can see the most popular GraphQL Client and probably has been for years now is StrawberryShake which also supports generating a client from an endpoint.

https://chillicream.com/docs/strawberryshake/v13/get-started/console

Would this be used in the long run?

@MikaelElkiaer
Copy link

@wingyplus @pjmagee I continued a bit on code generation here: https://github.com/MikaelElkiaer/dagger/tree/feat/sdk-dotnet-codegen/sdk/dotnet
I considered a bunch of alternatives, but ended up with https://github.com/CodegenCS/CodegenCS/ based on its conciseness and utility.
I did also consider Roslyn, but do not like that API at all.
The guy behind CodegenCS has some good reads with comparisons: CodegenCS vs Roslyn, Blog Post.

But I am considering abanding what I have done so far as I am considering alternatives to C#.
Feel free to use it, either way I will let it be.

@wingyplus
Copy link
Contributor Author

wingyplus commented Jun 1, 2024

@MikaelElkiaer that sound interesting. I will take a look when I'm online on my computer.

By the way, @pjmagee and I are discuss on codegen tool in Dagger discord in dotnet channel, you can reach us there.

@pjmagee
Copy link
Contributor

pjmagee commented Jun 1, 2024

@wingyplus @pjmagee I continued a bit on code generation here: https://github.com/MikaelElkiaer/dagger/tree/feat/sdk-dotnet-codegen/sdk/dotnet I considered a bunch of alternatives, but ended up with https://github.com/CodegenCS/CodegenCS/ based on its conciseness and utility. I did also consider Roslyn, but do not like that API at all. The guy behind CodegenCS has some good reads with comparisons: CodegenCS vs Roslyn, Blog Post.

But I am considering abanding what I have done so far as I am considering alternatives to C#. Feel free to use it, either way I will let it be.

The reason I would still recommend the Roslyn route because it's now a first class citizen as a source generator for .NET.

@pjmagee
Copy link
Contributor

pjmagee commented Jun 2, 2024

I was also looking into the SDK Docs on how to build an SDK. I can see the most popular GraphQL Client and probably has been for years now is StrawberryShake which also supports generating a client from an endpoint.

https://chillicream.com/docs/strawberryshake/v13/get-started/console

Would this be used in the long run?

Didnt look to be very friendly with how we would want to interact with dagger graphql. So not looking good unless others also can take a look into strawberry to see if iwe can still benefit from it? Otherwise I am not thinking its suitable anymore from a couple of hours trying to use it with dagger

@pjmagee
Copy link
Contributor

pjmagee commented Jun 3, 2024

@wingyplus Is it worth re-opening with latest changes we have and see if getting this current version merge into Dagger or we wait to make it bit more stable?

@wingyplus
Copy link
Contributor Author

@wingyplus Is it worth re-opening with latest changes we have and see if getting this current version merge into Dagger or we wait to make it bit more stable?

I recommended to opening the PR or discuss with the core team after we make the module supported. And we can iterate things (fix the api, review code, fix bugs) faster than living in the official repository.

@wingyplus
Copy link
Contributor Author

I believe we can close this PR for now and continue discuss on Dagger dotnet discord channel or open an issue at https://github.com/wingyplus/dagger-dotnet-sdk/issues. :)

@wingyplus wingyplus closed this Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants